Skip to content

Add CachingLocator. - #14020

Merged
Eric Snow (ericsnowcurrently) merged 39 commits into
microsoft:mainfrom
ericsnowcurrently:pyenvs-component-caching-locator
Oct 8, 2020
Merged

Add CachingLocator.#14020
Eric Snow (ericsnowcurrently) merged 39 commits into
microsoft:mainfrom
ericsnowcurrently:pyenvs-component-caching-locator

Conversation

@ericsnowcurrently

@ericsnowcurrently Eric Snow (ericsnowcurrently) commented Sep 21, 2020

Copy link
Copy Markdown

Note that I factored out the BackgroundRequestLooper class to keep CachingLocator focused, especially as I found it was important to deal with one "refresh" at a time.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • [ ] Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • [ ] Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • [ ] Test plan is updated as appropriate.
  • [ ] package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • [ ] The wiki is updated with any design decisions/details.

Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/index.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/collection/cachingLocator.ts Outdated
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Sep 29, 2020

Copy link
Copy Markdown

Codecov Report

Merging #14020 into main will increase coverage by 0.07%.
The diff coverage is 64.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #14020      +/-   ##
==========================================
+ Coverage   59.96%   60.04%   +0.07%     
==========================================
  Files         706      710       +4     
  Lines       39180    39420     +240     
  Branches     5681     5708      +27     
==========================================
+ Hits        23494    23668     +174     
- Misses      14449    14501      +52     
- Partials     1237     1251      +14     
Impacted Files Coverage Δ
src/client/pythonEnvironments/index.ts 17.64% <21.42%> (+0.57%) ⬆️
...ronments/base/locators/composite/cachingLocator.ts 64.88% <64.88%> (ø)
src/client/pythonEnvironments/base/envsCache.ts 92.59% <100.00%> (+8.59%) ⬆️
...onments/base/locators/composite/reducingLocator.ts 100.00% <100.00%> (ø)
src/client/common/utils/platform.ts 68.00% <0.00%> (-4.00%) ⬇️
...discovery/locators/services/windowsStoreLocator.ts 100.00% <0.00%> (ø)
src/client/pythonEnvironments/common/posixUtils.ts 91.66% <0.00%> (ø)
...covery/locators/services/posixKnownPathsLocator.ts 100.00% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef44f29...abf86c3. Read the comment docs.

@ericsnowcurrently
Eric Snow (ericsnowcurrently) marked this pull request as ready for review September 29, 2020 02:01
@ericsnowcurrently

Copy link
Copy Markdown
Author

Kim-Adeline Miguel (@kimadeline) and Kartik Raj (@karrtikr), I should have left this as a draft PR. I'll let you know when I've wrapped up the last few changes I have to make.

@ericsnowcurrently

Copy link
Copy Markdown
Author

Kim-Adeline Miguel (@kimadeline), Kartik Raj (@karrtikr), the PR is ready to go now.

Comment thread src/client/pythonEnvironments/base/cache.ts Outdated
Comment thread src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/base/cache.ts Outdated
Comment thread src/client/pythonEnvironments/index.ts Outdated
Comment thread src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts Outdated
Comment thread src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts Outdated
@kimadeline

Copy link
Copy Markdown

Since Kartik is out, you may want to request a review from Karthik Nadig (@karthiknadig), because I don't want to shoulder the responsibility of reviewing this PR alone 😆

@karrtikr

Copy link
Copy Markdown

Eric Snow (@ericsnowcurrently) Kim-Adeline Miguel (@kimadeline) I just had one comment regarding potential correctness #14020 (comment) I leave the rest upto you guys😄

@kimadeline

Copy link
Copy Markdown

Will re-review this afternoon.

Comment thread src/client/common/utils/backgroundLoop.ts Outdated
@ericsnowcurrently

Copy link
Copy Markdown
Author

resetting "checks" trigger

@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2020

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #14020 into main will decrease coverage by 0.00%.
The diff coverage is 64.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #14020      +/-   ##
==========================================
- Coverage   59.91%   59.91%   -0.01%     
==========================================
  Files         709      712       +3     
  Lines       39334    39520     +186     
  Branches     5698     5722      +24     
==========================================
+ Hits        23567    23678     +111     
- Misses      14529    14586      +57     
- Partials     1238     1256      +18     
Impacted Files Coverage Δ
src/client/pythonEnvironments/index.ts 17.64% <21.42%> (+0.57%) ⬆️
src/client/common/utils/backgroundLoop.ts 60.71% <60.71%> (ø)
...ronments/base/locators/composite/cachingLocator.ts 70.45% <70.45%> (ø)
src/client/pythonEnvironments/base/envsCache.ts 85.18% <84.61%> (+1.18%) ⬆️
...onments/base/locators/composite/reducingLocator.ts 100.00% <100.00%> (ø)
...very/locators/services/virtualenvwrapperLocator.ts 76.92% <0.00%> (-15.39%) ⬇️
src/client/common/utils/platform.ts 60.00% <0.00%> (-12.00%) ⬇️
.../pythonEnvironments/common/externalDependencies.ts 64.10% <0.00%> (-2.57%) ⬇️
src/client/linters/pydocstyle.ts 86.66% <0.00%> (-2.23%) ⬇️
src/client/datascience/debugLocationTracker.ts 76.56% <0.00%> (-1.57%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fee7e74...fa63318. Read the comment docs.

@ericsnowcurrently
Eric Snow (ericsnowcurrently) merged commit bc1d780 into microsoft:main Oct 8, 2020
@ericsnowcurrently
Eric Snow (ericsnowcurrently) deleted the pyenvs-component-caching-locator branch October 8, 2020 19:54
Luciana Abud (luabud) pushed a commit to luabud/vscode-python that referenced this pull request Oct 26, 2020
Note that we factored out the BackgroundRequestLooper class to keep CachingLocator focused, especially as it was important to deal with one "refresh" at a time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants